OCPBUGS-100298: pin internal-lb-monitor pollers to worker nodes - #31466
OCPBUGS-100298: pin internal-lb-monitor pollers to worker nodes#31466mkowalski wants to merge 1 commit into
Conversation
The internal-lb disruption poller tolerates the control-plane NoSchedule taint but has no nodeSelector, so when a replica is evicted mid-run (for example by the NoExecuteTaintManager serial tests tainting the worker it runs on) its replacement can be scheduled onto a control-plane node. On GCP that vantage point produces false internal-lb new-connections disruption on every kube-apiserver rollout of that node: apiserver-watcher removes the local VIP redirect ~15s into the graceful shutdown while the GCP passthrough LB health check only deprograms the backend ~25-30s in, and during the gap the node's own new connections to the api-int VIP are blackholed (backend VMs cannot reach their own VIP through the fabric). Reused connections survive via conntrack, so only new-connections backends are affected, matching the regression reported in OCPBUGS-100298. Pin the pollers to worker nodes so the measurement reflects the client path users actually take. The master toleration is kept so the pods stay schedulable on compact and single-node clusters, where control-plane nodes also carry the worker role. Assisted-By: Claude Fable 5
|
Pipeline controller notification For optional jobs, comment This repository is configured in: automatic mode |
openshift-ci-robot
added
the
jira/valid-reference
|
@mkowalski: This pull request references Jira Issue OCPBUGS-100298, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
openshift-ci-robot
added
the
jira/invalid-bug
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mkowalski The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
WalkthroughThe deployment manifest now schedules internal load-balancer pollers on worker nodes. It retains master-node toleration for compact clusters and documents the control-plane hairpin behavior. ChangesInternal load-balancer placement
Estimated code review effort: 1 (Trivial) | ~3 minutes Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error, 1 warning)
✅ Passed checks (13 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
/payload-job periodic-ci-openshift-release-main-nightly-5.0-e2e-gcp-ovn-serial |
1 similar comment
|
/payload-job periodic-ci-openshift-release-main-nightly-5.0-e2e-gcp-ovn-serial |
|
@mkowalski: trigger 5 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/53806120-8cf3-11f1-8f6c-5cf00301b374-0 |
openshift-ci-robot
added
the
jira/valid-bug
|
@mkowalski: This pull request references Jira Issue OCPBUGS-100298, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
openshift-ci-robot
removed
the
jira/invalid-bug
|
@mkowalski: trigger 5 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/5700ae40-8cf3-11f1-9454-c054eb1e913c-0 |
|
/jira refresh |
|
@mkowalski: This pull request references Jira Issue OCPBUGS-100298, which is valid. 3 validation(s) were run on this bug
DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
/payload-abort |
|
@mkowalski: Failed to abort some payload jobs. Total jobs aborted: 5. Failed jobs: 0aef3897-5627-467b-a723-74a9f004557e |
|
/payload-aggregate periodic-ci-openshift-release-main-nightly-5.0-e2e-gcp-ovn-serial 10 |
|
@mkowalski: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/8f55ea30-8cf3-11f1-96f9-913f299bfc92-0 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@pkg/monitortests/kubeapiserver/disruptioninclusterapiserver/manifests/dep-internal-lb.yaml`:
- Around line 70-71: Update the deployment strategy associated with the
nodeSelector in the internal load balancer manifest to prevent a single-worker
rollout deadlock: use Recreate, or configure RollingUpdate with maxUnavailable
set to 1 and maxSurge set to 0. Preserve the existing hostname anti-affinity
behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: e6cbc6ab-eb42-4756-949e-f7475d3d13af
📒 Files selected for processing (1)
pkg/monitortests/kubeapiserver/disruptioninclusterapiserver/manifests/dep-internal-lb.yaml
| nodeSelector: | ||
| node-role.kubernetes.io/worker: "" |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
target='pkg/monitortests/kubeapiserver/disruptioninclusterapiserver/manifests/dep-internal-lb.yaml'
printf '%s\n' '--- target files ---'
git ls-files 'pkg/monitortests/kubeapiserver/disruptioninclusterapiserver/manifests/*'
printf '%s\n' '--- target outline/size ---'
wc -l "$target"
printf '%s\n' '--- target manifest ---'
cat -n "$target"
printf '%s\n' '--- related anti-affinity, strategy, topology, and selector references ---'
rg -n -C 4 'dep-internal-lb|podAntiAffinity|topologyKey|maxUnavailable|maxSurge|node-role\.kubernetes\.io/worker|replicas:' \
pkg/monitortests/kubeapiserver/disruptioninclusterapiserver \
pkg/monitortests/kubeapiserver 2>/dev/null | head -n 500Repository: openshift/origin
Length of output: 34076
🏁 Script executed:
#!/bin/bash
set -eu
target='pkg/monitortests/kubeapiserver/disruptioninclusterapiserver/manifests/dep-internal-lb.yaml'
printf '%s\n' '--- target files ---'
git ls-files 'pkg/monitortests/kubeapiserver/disruptioninclusterapiserver/manifests/*'
printf '%s\n' '--- target manifest ---'
cat -n "$target"
printf '%s\n' '--- related scheduling references ---'
rg -n -C 4 'dep-internal-lb|podAntiAffinity|topologyKey|maxUnavailable|maxSurge|node-role\.kubernetes\.io/worker|replicas:' \
pkg/monitortests/kubeapiserver/disruptioninclusterapiserver \
pkg/monitortests/kubeapiserver 2>/dev/null | head -n 500Repository: openshift/origin
Length of output: 33917
🏁 Script executed:
#!/bin/bash
set -eu
target='pkg/monitortests/kubeapiserver/disruptioninclusterapiserver/manifests/dep-internal-lb.yaml'
python3 - "$target" <<'PY'
import re
import sys
from decimal import Decimal, ROUND_CEILING, ROUND_FLOOR
path = sys.argv[1]
text = open(path, encoding="utf-8").read()
def scalar(pattern, name):
match = re.search(pattern, text, re.MULTILINE)
if not match:
raise SystemExit(f"missing {name}")
return match.group(1)
replicas = int(scalar(r"^ replicas:\s+(\d+)\s*$", "replicas"))
strategy = re.search(r"^ strategy:\s*$", text, re.MULTILINE)
anti_affinity = bool(re.search(
r"requiredDuringSchedulingIgnoredDuringExecution:.*?"
r"topologyKey:\s*[\"']kubernetes\.io/hostname[\"']",
text, re.DOTALL))
worker_selector = bool(re.search(
r"^ nodeSelector:\s*\n"
r"^ node-role\.kubernetes\.io/worker:\s*[\"']?[\"']?\s*$",
text, re.MULTILINE))
def percent(value, replicas, rounding):
number = Decimal(value.rstrip("%")) * replicas / Decimal(100)
return int(number.to_integral_value(rounding=rounding))
# Deployment defaults: maxSurge=25% rounds up; maxUnavailable=25% rounds down.
max_surge = percent("25%", replicas, ROUND_CEILING)
max_unavailable = percent("25%", replicas, ROUND_FLOOR)
print({
"replicas": replicas,
"explicit_strategy": bool(strategy),
"required_hostname_anti_affinity": anti_affinity,
"worker_selector": worker_selector,
"default_maxSurge": max_surge,
"default_maxUnavailable": max_unavailable,
"single_eligible_node_replacement_pending": (
replicas == 1 and anti_affinity and worker_selector
and max_surge >= 1 and max_unavailable == 0
),
})
assert replicas == 1
assert not strategy
assert anti_affinity
assert worker_selector
assert max_surge == 1
assert max_unavailable == 0
assert replicas == 1 and anti_affinity and worker_selector
PY
printf '%s\n' '--- target file change relative to parent ---'
git diff --stat HEAD^ -- "$target" 2>/dev/null || true
git diff --unified=3 HEAD^ -- "$target" 2>/dev/null | head -n 160 || true
printf '%s\n' '--- explicit rollout strategies in related manifests ---'
rg -n -C 3 '^[[:space:]]+strategy:|maxUnavailable:|maxSurge:' \
pkg/monitortests/kubeapiserver/disruptioninclusterapiserver/manifestsRepository: openshift/origin
Length of output: 3849
🏁 Script executed:
#!/bin/bash
set -eu
target='pkg/monitortests/kubeapiserver/disruptioninclusterapiserver/manifests/dep-internal-lb.yaml'
python3 - "$target" <<'PY'
import re
import sys
from decimal import Decimal, ROUND_CEILING, ROUND_FLOOR
path = sys.argv[1]
text = open(path, encoding="utf-8").read()
def scalar(pattern, name):
match = re.search(pattern, text, re.MULTILINE)
if not match:
raise SystemExit(f"missing {name}")
return match.group(1)
replicas = int(scalar(r"^ replicas:\s+(\d+)\s*$", "replicas"))
strategy = re.search(r"^ strategy:\s*$", text, re.MULTILINE)
anti_affinity = bool(re.search(
r"requiredDuringSchedulingIgnoredDuringExecution:.*?"
r"topologyKey:\s*[\"']kubernetes\.io/hostname[\"']",
text, re.DOTALL))
worker_selector = bool(re.search(
r"^ nodeSelector:\s*\n"
r"^ node-role\.kubernetes\.io/worker:\s*[\"']?[\"']?\s*$",
text, re.MULTILINE))
def percent(value, replicas, rounding):
number = Decimal(value.rstrip("%")) * replicas / Decimal(100)
return int(number.to_integral_value(rounding=rounding))
max_surge = percent("25%", replicas, ROUND_CEILING)
max_unavailable = percent("25%", replicas, ROUND_FLOOR)
print({
"replicas": replicas,
"explicit_strategy": bool(strategy),
"required_hostname_anti_affinity": anti_affinity,
"worker_selector": worker_selector,
"default_maxSurge": max_surge,
"default_maxUnavailable": max_unavailable,
"single_eligible_node_replacement_pending": (
replicas == 1 and anti_affinity and worker_selector
and max_surge >= 1 and max_unavailable == 0
),
})
assert replicas == 1
assert not strategy
assert anti_affinity
assert worker_selector
assert max_surge == 1
assert max_unavailable == 0
PY
printf '%s\n' '--- target file change relative to parent ---'
git diff --stat HEAD^ -- "$target" 2>/dev/null || true
git diff --unified=3 HEAD^ -- "$target" 2>/dev/null | head -n 160 || true
printf '%s\n' '--- explicit rollout strategies in related manifests ---'
rg -n -C 3 '^[[:space:]]+strategy:|maxUnavailable:|maxSurge:' \
pkg/monitortests/kubeapiserver/disruptioninclusterapiserver/manifestsRepository: openshift/origin
Length of output: 3849
Prevent a single-node rollout deadlock.
If only one worker node is eligible, required hostname anti-affinity blocks the replacement Pod. With one replica, the default RollingUpdate strategy sets maxUnavailable to 0, so the old Pod remains and the rollout stays Pending.
Set maxUnavailable: 1 and maxSurge: 0, use Recreate, or make anti-affinity preferred.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@pkg/monitortests/kubeapiserver/disruptioninclusterapiserver/manifests/dep-internal-lb.yaml`
around lines 70 - 71, Update the deployment strategy associated with the
nodeSelector in the internal load balancer manifest to prevent a single-worker
rollout deadlock: use Recreate, or configure RollingUpdate with maxUnavailable
set to 1 and maxSurge set to 0. Preserve the existing hostname anti-affinity
behavior.
Sources: Coding guidelines, MCP tools
|
Scheduling required tests: |
Validation results: 10×
|
| Run (UTC) | ilb-new | localhost | poller displaced to master? |
|---|---|---|---|
| Jul 25 18:57 | 15 | 48 | yes (master) |
| Jul 26 02:24 | 27 | ~49 | yes (master-2) |
| Jul 26 09:11 | 19 | — | yes |
| Jul 26 16:39 | 0 | 53 | no (replacement → worker-f) |
| Jul 26 22:34 | 19 | — | yes (master-0) |
| Jul 27 05:43 | 24 | — | yes |
| Jul 27 11:22 | 1 | 49 | no (stayed on workers) |
| Jul 27 17:18 | 20 | — | yes |
| Jul 29 00:08 | 22 | — | yes |
| Jul 29 06:06 | 13 | — | yes |
| Mean / affected | 16.0 | 8/10 runs affected |
With PR — 10× openshift-origin-31466-nightly-5.0-e2e-gcp-ovn-serial (aggregation 309ea076…)
| Run | Job result | ilb-new | ilb-reused | localhost |
|---|---|---|---|---|
| 2083211564936073216 | success | 0 | 0 | 50 |
| 2083211565372280832 | success | 0 | 0 | 52 |
| 2083211565833654272 | success | 0 | 5 | 58 |
| 2083211566274056192 | failure* | 0 | 0 | 48 |
| 2083211566731235328 | success | 0 | 0 | 48 |
| 2083211567205191680 | success | 0 | 0 | 51 |
| 2083211567628816384 | failure* | 0 | 0 | 49 |
| 2083211568090189824 | failure* | 0 | 0 | 53 |
| 2083211568547368960 | failure* | 0 | 0 | 55 |
| 2083211568975187968 | failure* | 0 | 1 | 51 |
| Mean / affected | 0.0 | 0/10 runs affected |
* job-level failures are unrelated MonitorTest flakes (e.g. kubelet-container-restarts, KubePodNotReady alert invariants), not disruption.
Mechanism confirmed end-to-end
In run 2083211568975187968 the exact trigger scenario replayed with the fix in place: the NoExecuteTaintManager Multiple Pods test (16:59–17:01) evicted an internal-lb-monitor replica, and the replacement was scheduled to worker-c (nodeSelector now forbids masters) instead of a control-plane node; the external ip policy test then ran its two kube-apiserver rollouts (18:16–18:38) with zero internal-LB new-connections disruption.
Summary: without the PR 8/10 runs showed 13–27 false disruption events (mean 16.0); with the PR 0/10 runs showed any (0.0), while the rollout pattern (localhost events 48–58) was identical.
This comment was generated using AI (Assisted-By: Claude Fable 5). Please verify before acting on it.
2 participants
Fixes the internal-LB new-connections "disruption regression" tracked in OCPBUGS-100298.
What was happening
The
internal-lb-monitorpoller deployment tolerates the control-planeNoScheduletaint but has no nodeSelector. Its replicas normally land on workers, but when a replica is evicted mid-run — the[sig-node] NoExecuteTaintManager Multiple Pods [Serial]test taints the worker hosting it — the replacement is scheduled ~1s later while the taint is still active, and if no untainted worker is free (anti-affinity holds the others), it lands on a control-plane node.On GCP that vantage point measures a platform hairpin gap instead of user-perceived availability. During every kube-apiserver graceful shutdown on that node:
/readyzgoes red at T+1sapiserver-watcherwrites the VIP downfile at T+15s (8 × 2s failures) →gcp-routesremoves the local VIP DNATCheckIntervalSec: 5×UnhealthyThreshold: 6)Result: 2–12 disruption events per internal-lb new-connections backend per run, in two bursts (the hosting master rolls twice during the
ensures external ip policytest). Verified across 7 runs ofperiodic-ci-openshift-release-main-nightly-5.0-e2e-gcp-ovn-serial: disruption occurs iff the replacement replica landed on a master (audit-log source IPs, scheduling events, gcp-routes journals and apiserver-watcher logs all line up; full evidence in the Jira). The previously suspected cluster-kube-apiserver-operator#2247 and openshift/kubernetes#2713 are exonerated — the payload correlation was scheduler-placement luck.The fix
Pin the internal-lb pollers to worker nodes with a
nodeSelector. The master toleration is kept so the pods remain schedulable on compact/SNO clusters where control-plane nodes also carry the worker role.This PR was generated using AI (Assisted-By: Claude Fable 5). Please verify before acting on it.
Summary by CodeRabbit